Skip to content

fix: use more strict compiler flags #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
anonrig merged 1 commit into from
Feb 4, 2026
Merged

fix: use more strict compiler flags #34

anonrig merged 1 commit into from
Feb 4, 2026
+30 −1

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Feb 3, 2026

Enables more strict compiler flags to avoid warnings like this:

external/+http+ncrypto/include/ncrypto/aead.h:22:54: warning: field 'info_' will be initialized after field 'aead_' [-Wreorder-ctor]
   22 |   Aead(const AeadInfo* info, const EVP_AEAD* aead) : info_(info), aead_(aead) {}
      |                                                      ^~~~~~~~~~~  ~~~~~~~~~~~
      |                                                      aead_(aead)  info_(info)
1 warning generated.

@anonrig anonrig requested review from aduh95 and jasnell February 3, 2026 16:24
anonrig
anonrig commented Feb 3, 2026
View reviewed changes
@anonrig anonrig force-pushed the yagiz/more-compiler-checks branch 2 times, most recently from 56cf070 to 4300192 Compare February 3, 2026 16:36
@anonrig anonrig force-pushed the yagiz/more-compiler-checks branch from 4300192 to d628ab0 Compare February 3, 2026 17:43
@anonrig
Copy link
Member Author

anonrig commented Feb 3, 2026

cc @jasnell @aduh95 can you review?

aduh95
aduh95 approved these changes Feb 4, 2026
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about MSVC flags, the rest LGTM

targos
targos reviewed Feb 4, 2026
View reviewed changes
src/CMakeLists.txt
/wd4100 # Unreferenced formal parameter (like -Wno-unused-parameter)
/wd4267 # Conversion from 'size_t' to smaller type
/wd4244 # Conversion, possible loss of data
/wd4305 # Truncation from 'int' to 'bool'
Copy link
Member

@targos targos Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it make sense to fix these?

Copy link
Member Author

@anonrig anonrig Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll follow up.

@anonrig anonrig merged commit fc401e3 into main Feb 4, 2026
13 checks passed
@anonrig anonrig deleted the yagiz/more-compiler-checks branch February 4, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targos targos targos left review comments

@aduh95 aduh95 aduh95 approved these changes

@jasnell jasnell Awaiting requested review from jasnell

+1 more reviewer

@harrishancock harrishancock harrishancock approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anonrig @harrishancock @targos @aduh95